Propose fix for connection testing issues in Cloud. #836
Conversation
VersusFacit
requested review from
andrefurlan-db,
benc-db and
rcypher-databricks
as code owners
| try: | ||
| if self.behavior.use_info_schema_for_columns.no_warn: # type: ignore[attr-defined] | ||
| self.get_column_behavior = GetColumnsByInformationSchema() | ||
| except CompilationError: |
There was a problem hiding this comment.
we didn't expect behavior flags to be used this early in the runtime, so we need an extra check for the use here that protects against situations where flags don't get registered
There was a problem hiding this comment.
I would move this condition into get_columns_in_relation. It looks like that's the only thing that references self.get_column"_behavior (at least in the adapter itself). And if we're at that point, then we definitely have flags from project.yml. This also aligns with pushing behavior flags as close to their relevant code as possible. While column metadata is a part of the vast majority of runs, we should still strive to keep the flag close to the code that it impacts, that way when we remove the flag we have a better idea of what we're affecting.
There was a problem hiding this comment.
Why is this not defined by init time? In general I try to follow the pattern of https://blog.ploeh.dk/2011/07/28/CompositionRoot/, but man does dbt make that difficult. I would strongly prefer not to delay selecting behavior until get_columns_in_relation. I am fine with @VersusFacit's suggestion, but don't really understand why the initialization behavior is not the same for all paths.
There was a problem hiding this comment.
We took this thread to a more rapid response forum but for posterity project flags come from the dbt_project.yml which cloud doesn't always have in scope for certain workflows. So for that situation, we pass a dummy dbt_project.yml (empty of flags) and use profiles.yml / the cloud equivalent to, say, connection test.
| try: | ||
| if self.behavior.use_info_schema_for_columns.no_warn: # type: ignore[attr-defined] | ||
| self.get_column_behavior = GetColumnsByInformationSchema() | ||
| except CompilationError: |
There was a problem hiding this comment.
Why is this not defined by init time? In general I try to follow the pattern of https://blog.ploeh.dk/2011/07/28/CompositionRoot/, but man does dbt make that difficult. I would strongly prefer not to delay selecting behavior until get_columns_in_relation. I am fine with @VersusFacit's suggestion, but don't really understand why the initialization behavior is not the same for all paths.
…t.yml is irrelevant
Description
dbt cloud has a feature for testing connections. It and other features are currently failing due to adapter behavior flags not being registered for those runs.
If we look at these lines, we see the use of a flag in the adapter class init. This was par t of #808
This is not guaranteed to succeed. So we can use exception handling to ensure the control flow for other subtasks where the flags won't be registered are handled gracefully.
Checklist
CHANGELOG.mdand added information about my change to the "dbt-databricks next" section.